Skip to content

[flang][openacc] Allow finalize clause on exit data more than once #135415

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 11, 2025

Conversation

clementval
Copy link
Contributor

The spec has not strict restriction to allow a single finalize clause on the exit data directive. Allowing this clause multiple times does not change the semantic of it. This patch relax the rules in ACC.td since there is no restriction in the standard.

The OpenACC dialect represent the finalize clause with a UnitAttr so the attribute will be set if the is one or more finalize clause.

@llvmbot llvmbot added flang Flang issues not falling into any other category flang:fir-hlfir openacc flang:semantics labels Apr 11, 2025
@llvmbot
Copy link
Member

llvmbot commented Apr 11, 2025

@llvm/pr-subscribers-openacc

@llvm/pr-subscribers-flang-fir-hlfir

Author: Valentin Clement (バレンタイン クレメン) (clementval)

Changes

The spec has not strict restriction to allow a single finalize clause on the exit data directive. Allowing this clause multiple times does not change the semantic of it. This patch relax the rules in ACC.td since there is no restriction in the standard.

The OpenACC dialect represent the finalize clause with a UnitAttr so the attribute will be set if the is one or more finalize clause.


Full diff: https://github.com/llvm/llvm-project/pull/135415.diff

3 Files Affected:

  • (modified) flang/test/Lower/OpenACC/acc-exit-data.f90 (+5)
  • (modified) flang/test/Semantics/OpenACC/acc-data.f90 (+1-1)
  • (modified) llvm/include/llvm/Frontend/OpenACC/ACC.td (+4-8)
diff --git a/flang/test/Lower/OpenACC/acc-exit-data.f90 b/flang/test/Lower/OpenACC/acc-exit-data.f90
index fbd1edfb6afe4..bf5f7094913a1 100644
--- a/flang/test/Lower/OpenACC/acc-exit-data.f90
+++ b/flang/test/Lower/OpenACC/acc-exit-data.f90
@@ -102,4 +102,9 @@ subroutine acc_exit_data
 !CHECK: acc.exit_data wait_devnum(%[[WAIT6]] : i32) wait(%[[WAIT4]], %[[WAIT5]] : i32, i32) dataOperands(%[[DEVPTR]] : !fir.ref<!fir.array<10x10xf32>>)
 !CHECK: acc.delete accPtr(%[[DEVPTR]] : !fir.ref<!fir.array<10x10xf32>>) {name = "a", structured = false}
 
+  !$acc exit data delete(a) finalize
+!CHECK: acc.exit_data dataOperands(%{{.*}} : !fir.ref<!fir.array<10x10xf32>>) attributes {finalize}
+
+  !$acc exit data delete(a) finalize finalize
+!CHECK: acc.exit_data dataOperands(%{{.*}} : !fir.ref<!fir.array<10x10xf32>>) attributes {finalize}
 end subroutine acc_exit_data
diff --git a/flang/test/Semantics/OpenACC/acc-data.f90 b/flang/test/Semantics/OpenACC/acc-data.f90
index 84eb72825b34c..ac49532c495d5 100644
--- a/flang/test/Semantics/OpenACC/acc-data.f90
+++ b/flang/test/Semantics/OpenACC/acc-data.f90
@@ -86,7 +86,7 @@ program openacc_data_validity
 
   !$acc exit data delete(aa) finalize
 
-  !ERROR: At most one FINALIZE clause can appear on the EXIT DATA directive
+  ! OK
   !$acc exit data delete(aa) finalize finalize
 
   !ERROR: Argument `cc` on the DETACH clause must be a variable or array with the POINTER or ALLOCATABLE attribute
diff --git a/llvm/include/llvm/Frontend/OpenACC/ACC.td b/llvm/include/llvm/Frontend/OpenACC/ACC.td
index 8729d4505205b..cee0d53458a08 100644
--- a/llvm/include/llvm/Frontend/OpenACC/ACC.td
+++ b/llvm/include/llvm/Frontend/OpenACC/ACC.td
@@ -535,14 +535,10 @@ def ACC_EnterData : Directive<"enter data"> {
 
 // 2.14.7
 def ACC_ExitData : Directive<"exit data"> {
-  let allowedClauses = [
-    VersionedClause<ACCC_Wait>
-  ];
-  let allowedOnceClauses = [
-    VersionedClause<ACCC_Async>,
-    VersionedClause<ACCC_If>,
-    VersionedClause<ACCC_Finalize>
-  ];
+  let allowedClauses = [VersionedClause<ACCC_Finalize>,
+                        VersionedClause<ACCC_Wait>];
+  let allowedOnceClauses = [VersionedClause<ACCC_Async>,
+                            VersionedClause<ACCC_If>];
   let requiredClauses = [
     VersionedClause<ACCC_Copyout>,
     VersionedClause<ACCC_Delete>,

@llvmbot
Copy link
Member

llvmbot commented Apr 11, 2025

@llvm/pr-subscribers-flang-semantics

Author: Valentin Clement (バレンタイン クレメン) (clementval)

Changes

The spec has not strict restriction to allow a single finalize clause on the exit data directive. Allowing this clause multiple times does not change the semantic of it. This patch relax the rules in ACC.td since there is no restriction in the standard.

The OpenACC dialect represent the finalize clause with a UnitAttr so the attribute will be set if the is one or more finalize clause.


Full diff: https://github.com/llvm/llvm-project/pull/135415.diff

3 Files Affected:

  • (modified) flang/test/Lower/OpenACC/acc-exit-data.f90 (+5)
  • (modified) flang/test/Semantics/OpenACC/acc-data.f90 (+1-1)
  • (modified) llvm/include/llvm/Frontend/OpenACC/ACC.td (+4-8)
diff --git a/flang/test/Lower/OpenACC/acc-exit-data.f90 b/flang/test/Lower/OpenACC/acc-exit-data.f90
index fbd1edfb6afe4..bf5f7094913a1 100644
--- a/flang/test/Lower/OpenACC/acc-exit-data.f90
+++ b/flang/test/Lower/OpenACC/acc-exit-data.f90
@@ -102,4 +102,9 @@ subroutine acc_exit_data
 !CHECK: acc.exit_data wait_devnum(%[[WAIT6]] : i32) wait(%[[WAIT4]], %[[WAIT5]] : i32, i32) dataOperands(%[[DEVPTR]] : !fir.ref<!fir.array<10x10xf32>>)
 !CHECK: acc.delete accPtr(%[[DEVPTR]] : !fir.ref<!fir.array<10x10xf32>>) {name = "a", structured = false}
 
+  !$acc exit data delete(a) finalize
+!CHECK: acc.exit_data dataOperands(%{{.*}} : !fir.ref<!fir.array<10x10xf32>>) attributes {finalize}
+
+  !$acc exit data delete(a) finalize finalize
+!CHECK: acc.exit_data dataOperands(%{{.*}} : !fir.ref<!fir.array<10x10xf32>>) attributes {finalize}
 end subroutine acc_exit_data
diff --git a/flang/test/Semantics/OpenACC/acc-data.f90 b/flang/test/Semantics/OpenACC/acc-data.f90
index 84eb72825b34c..ac49532c495d5 100644
--- a/flang/test/Semantics/OpenACC/acc-data.f90
+++ b/flang/test/Semantics/OpenACC/acc-data.f90
@@ -86,7 +86,7 @@ program openacc_data_validity
 
   !$acc exit data delete(aa) finalize
 
-  !ERROR: At most one FINALIZE clause can appear on the EXIT DATA directive
+  ! OK
   !$acc exit data delete(aa) finalize finalize
 
   !ERROR: Argument `cc` on the DETACH clause must be a variable or array with the POINTER or ALLOCATABLE attribute
diff --git a/llvm/include/llvm/Frontend/OpenACC/ACC.td b/llvm/include/llvm/Frontend/OpenACC/ACC.td
index 8729d4505205b..cee0d53458a08 100644
--- a/llvm/include/llvm/Frontend/OpenACC/ACC.td
+++ b/llvm/include/llvm/Frontend/OpenACC/ACC.td
@@ -535,14 +535,10 @@ def ACC_EnterData : Directive<"enter data"> {
 
 // 2.14.7
 def ACC_ExitData : Directive<"exit data"> {
-  let allowedClauses = [
-    VersionedClause<ACCC_Wait>
-  ];
-  let allowedOnceClauses = [
-    VersionedClause<ACCC_Async>,
-    VersionedClause<ACCC_If>,
-    VersionedClause<ACCC_Finalize>
-  ];
+  let allowedClauses = [VersionedClause<ACCC_Finalize>,
+                        VersionedClause<ACCC_Wait>];
+  let allowedOnceClauses = [VersionedClause<ACCC_Async>,
+                            VersionedClause<ACCC_If>];
   let requiredClauses = [
     VersionedClause<ACCC_Copyout>,
     VersionedClause<ACCC_Delete>,

@clementval clementval merged commit 609361a into llvm:main Apr 11, 2025
16 checks passed
@clementval clementval deleted the acc_finalize branch April 11, 2025 20:58
clementval added a commit that referenced this pull request Apr 11, 2025
…ate (#135422)

Similar to #135415.

The spec has not strict restriction to allow a single `if_present`
clause on the host_data and update directives. Allowing this clause
multiple times does not change the semantic of it. This patch relax the
rules in ACC.td since there is no restriction in the standard.

The OpenACC dialect represents the `if_present` clause with a `UnitAttr`
so the attribute will be set if the is one or more `if_present` clause.
bcardosolopes added a commit to bcardosolopes/llvm-project that referenced this pull request Apr 11, 2025
* origin/main: (287 commits)
  [Sema] On Windows, silence erroneous warning when building with MSVC
  [lldb][lldbp-dap] On Windoows, silence warnings when building with MSVC
  [lldb] Fix erroneous return value
  [compiler-rt] On Windows, silence warning when building with Clang ToT
  [clang][unittests] On Windows, silence warning when building with MSVC
  [lldb] On Windows, silence warning when building with Clang ToT
  [CIR] Make LLVM & OGCG variables match the same pattern (llvm#135427)
  [mlir][SMT] upstream `SMT` dialect (llvm#131480)
  [clang] fix serialization for SubstNonTypeTemplateParmPackExpr (llvm#135428)
  [flang][openacc] Allow if_present multiple times on host_data and update (llvm#135422)
  [flang][openacc] Allow finalize clause on exit data more than once (llvm#135415)
  [flang] IEEE_SCALB and SCALE - kind=2, kind=3 (llvm#135374)
  [-Wunsafe-buffer-usage] Add findUnsafePointers (llvm#135421)
  [compiler-rt][sanitizer] add Haiku support (llvm#134772)
  [cpp23] Remove usage of std::aligned_union<> in llvm (llvm#135146)
  [mlir][tosa] Add error_if checks for Mul Op (llvm#135075)
  [VPlan] Merge cases using getResultType in inferScalarType (NFC).
  [flang][runtime] Fix recently broken big-endian formatted integer input (llvm#135417)
  [AMDGPU][Verifier] Mark calls to entry functions as invalid in the IR verifier (llvm#134910)
  [llvm][Hexagon] Promote operand v2i1 to v2i32 (llvm#135409)
  ...
var-const pushed a commit to ldionne/llvm-project that referenced this pull request Apr 17, 2025
…lvm#135415)

The spec has not strict restriction to allow a single `finalize` clause
on the `exit data` directive. Allowing this clause multiple times does
not change the semantic of it. This patch relax the rules in `ACC.td`
since there is no restriction in the standard.

The OpenACC dialect represent the finalize clause with a UnitAttr so the
attribute will be set if the is one or more `finalize` clause.
var-const pushed a commit to ldionne/llvm-project that referenced this pull request Apr 17, 2025
…ate (llvm#135422)

Similar to llvm#135415.

The spec has not strict restriction to allow a single `if_present`
clause on the host_data and update directives. Allowing this clause
multiple times does not change the semantic of it. This patch relax the
rules in ACC.td since there is no restriction in the standard.

The OpenACC dialect represents the `if_present` clause with a `UnitAttr`
so the attribute will be set if the is one or more `if_present` clause.
erichkeane added a commit that referenced this pull request Apr 18, 2025
#135372)

…uses

The Flang implemenation of OpenACC uses a .td file in the llvm/Frontend
directory to determine appertainment in 4 categories:

-Required: If this list has items in it, the directive requires at least
1 of these be present.

-AllowedExclusive: Items on this list are all allowed, but only 1 from
the list may be here (That is, they are exclusive of eachother).

-AllowedOnce: Items on this list are all allowed, but may not be
duplicated.

Allowed: Items on this list are allowed. Note th at the actual list of
'allowed' is all 4 of these lists together.

This is a draft patch to swtich Clang over to use those tables. Surgery
to get this to happen in Clang Sema was somewhat reasonable. However,
some gaps in the implementations are obvious, the existing clang
implementation disagrees with the Flang interpretation of it. SO, we're
keeping a task list here based on what gets discovered.

Changes to Clang:
- [x] Switch 'directive-kind' enum conversions to use tablegen See
ff1a7bd
- [x] Switch 'clause-kind' enum conversions to use tablegen See
ff1a7bd
- [x] Investigate 'parse' test differences to see if any new
disagreements arise.
- [x] Clang/Flang disagree as to whether 'collapse' can be multiple
times on a loop. Further research showed no prose to limit this, and the
comment on the clang implementation said "no good reason to allow", so
no standards justification.
- [x] Clang/Flang disagree whether 'num_gangs' can appear >1 on a
compute/combined construct. This ended up being an unjustified
restriction.
- [x] Clang/Flang disagree as to the list of required clauses on a 'set'
construct. My research shows that Clang mistakenly included 'if' in the
list, and that it should be just 'default_async', 'device_num', and
'device_type'.
- [x] Order of 'at least one of' diagnostic has changed. Tests were
updated.
- [x] Ensure we are properly 'de-aliasing' clause names in appertainment
checks?
- [x] What is 'shortloop'? 'shortloop' seems to be an old non-standard
extension that isn't supported by flang, but is parsed for backward
compat reasons. Clang won't parse, but we at least have a spot for it in
the clause list.
- [x] Implemented proposed change for 'routine' gang/worker/vector/seq.
(see issue 539)
- [x] Implement init/shutdown can only have 1 'if' (see issue 540)
- [x] Clang/Flang disagree as to whether 'tile' is permitted more than
once on a 'loop' or combined constructs (Flang prohibits >1). I see no
justification for this in the standard. EDIT: I found a comment in clang
that I did this to make SOMETHING around duplicate checks easier.
Discussion showed we should actually have a better behavior around
'device_type' and duplicates, so I've since implemented that.
- [x] Clang/Flang disagree whether 'gang', 'worker', or 'vector' may
appear on the same construct as a 'seq' on a 'loop' or 'combined'. There
is prose for this in 2022: (a gang, worker, or vector clause may not
appear if a 'seq' clause appears). EDIT: These don't actually disagree,
but aren't in the .td file, so I restored the existing code to do this.
- [x] Clang/Flang disagree on whether 'bind' can appear >1 on a
'routine'. I believe line 3096 (A bind clause may not bind to a routine
name that has a visible bind clause) makes this limitation (Flang
permits >1 bind). we discussed and decided this should have the same
rules as worker/vector/etc, except without the 'exactly 1 of' rule (so
no dupes in individual sections).
- [x] Clang/Flang disagree on whether 'init'/'shutdown' can have
multiple 'device_num' clauses. I believe there is no supporting prose
for this limitation., We decided that `device_num` should only happen
1x.
- [x] Clang/Flang disagree whether 'num_gangs' can appear >1 on a
'kernels' construct. Line 1173 (On a kernels construct, the num_gangs
clause must have a single argument) justifies limiting on a
per-arguement basis, but doesn't do so for multiple num_gangs clauses.
WE decided to do this with the '1-per-device-type' region for num_gangs,
num_workers, and vector_length, see openacc bug here:
OpenACC/openacc-spec#541

Changes to Flang:
- [x] Clang/Flang disgree on whether 'atomic' can take an 'if' clause.
This was added in OpenACC3.3_Next See #135451
- [x] Clang/Flang disagree on whether 'finalize' can be allowed >1 times
on a 'exit_data' construct. see #135415.
- [x] Clang/Flang disagree whether 'if_present' should be allowed >1
times on a 'host_data'/'update' construct. see #135422
- [x] Clang/Flang disagree on whether 'init'/'shutdown' can have
multiple 'device_type' clauses. I believe there is no supporting prose
for this limitation.
- [ ] SEE change for num_gangs/etc above.


Changes that need discussion/research:
IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
llvm#135372)

…uses

The Flang implemenation of OpenACC uses a .td file in the llvm/Frontend
directory to determine appertainment in 4 categories:

-Required: If this list has items in it, the directive requires at least
1 of these be present.

-AllowedExclusive: Items on this list are all allowed, but only 1 from
the list may be here (That is, they are exclusive of eachother).

-AllowedOnce: Items on this list are all allowed, but may not be
duplicated.

Allowed: Items on this list are allowed. Note th at the actual list of
'allowed' is all 4 of these lists together.

This is a draft patch to swtich Clang over to use those tables. Surgery
to get this to happen in Clang Sema was somewhat reasonable. However,
some gaps in the implementations are obvious, the existing clang
implementation disagrees with the Flang interpretation of it. SO, we're
keeping a task list here based on what gets discovered.

Changes to Clang:
- [x] Switch 'directive-kind' enum conversions to use tablegen See
ff1a7bd
- [x] Switch 'clause-kind' enum conversions to use tablegen See
ff1a7bd
- [x] Investigate 'parse' test differences to see if any new
disagreements arise.
- [x] Clang/Flang disagree as to whether 'collapse' can be multiple
times on a loop. Further research showed no prose to limit this, and the
comment on the clang implementation said "no good reason to allow", so
no standards justification.
- [x] Clang/Flang disagree whether 'num_gangs' can appear >1 on a
compute/combined construct. This ended up being an unjustified
restriction.
- [x] Clang/Flang disagree as to the list of required clauses on a 'set'
construct. My research shows that Clang mistakenly included 'if' in the
list, and that it should be just 'default_async', 'device_num', and
'device_type'.
- [x] Order of 'at least one of' diagnostic has changed. Tests were
updated.
- [x] Ensure we are properly 'de-aliasing' clause names in appertainment
checks?
- [x] What is 'shortloop'? 'shortloop' seems to be an old non-standard
extension that isn't supported by flang, but is parsed for backward
compat reasons. Clang won't parse, but we at least have a spot for it in
the clause list.
- [x] Implemented proposed change for 'routine' gang/worker/vector/seq.
(see issue 539)
- [x] Implement init/shutdown can only have 1 'if' (see issue 540)
- [x] Clang/Flang disagree as to whether 'tile' is permitted more than
once on a 'loop' or combined constructs (Flang prohibits >1). I see no
justification for this in the standard. EDIT: I found a comment in clang
that I did this to make SOMETHING around duplicate checks easier.
Discussion showed we should actually have a better behavior around
'device_type' and duplicates, so I've since implemented that.
- [x] Clang/Flang disagree whether 'gang', 'worker', or 'vector' may
appear on the same construct as a 'seq' on a 'loop' or 'combined'. There
is prose for this in 2022: (a gang, worker, or vector clause may not
appear if a 'seq' clause appears). EDIT: These don't actually disagree,
but aren't in the .td file, so I restored the existing code to do this.
- [x] Clang/Flang disagree on whether 'bind' can appear >1 on a
'routine'. I believe line 3096 (A bind clause may not bind to a routine
name that has a visible bind clause) makes this limitation (Flang
permits >1 bind). we discussed and decided this should have the same
rules as worker/vector/etc, except without the 'exactly 1 of' rule (so
no dupes in individual sections).
- [x] Clang/Flang disagree on whether 'init'/'shutdown' can have
multiple 'device_num' clauses. I believe there is no supporting prose
for this limitation., We decided that `device_num` should only happen
1x.
- [x] Clang/Flang disagree whether 'num_gangs' can appear >1 on a
'kernels' construct. Line 1173 (On a kernels construct, the num_gangs
clause must have a single argument) justifies limiting on a
per-arguement basis, but doesn't do so for multiple num_gangs clauses.
WE decided to do this with the '1-per-device-type' region for num_gangs,
num_workers, and vector_length, see openacc bug here:
https://github.com/OpenACC/openacc-spec/issues/541

Changes to Flang:
- [x] Clang/Flang disgree on whether 'atomic' can take an 'if' clause.
This was added in OpenACC3.3_Next See llvm#135451
- [x] Clang/Flang disagree on whether 'finalize' can be allowed >1 times
on a 'exit_data' construct. see llvm#135415.
- [x] Clang/Flang disagree whether 'if_present' should be allowed >1
times on a 'host_data'/'update' construct. see llvm#135422
- [x] Clang/Flang disagree on whether 'init'/'shutdown' can have
multiple 'device_type' clauses. I believe there is no supporting prose
for this limitation.
- [ ] SEE change for num_gangs/etc above.


Changes that need discussion/research:
IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
llvm#135372)

…uses

The Flang implemenation of OpenACC uses a .td file in the llvm/Frontend
directory to determine appertainment in 4 categories:

-Required: If this list has items in it, the directive requires at least
1 of these be present.

-AllowedExclusive: Items on this list are all allowed, but only 1 from
the list may be here (That is, they are exclusive of eachother).

-AllowedOnce: Items on this list are all allowed, but may not be
duplicated.

Allowed: Items on this list are allowed. Note th at the actual list of
'allowed' is all 4 of these lists together.

This is a draft patch to swtich Clang over to use those tables. Surgery
to get this to happen in Clang Sema was somewhat reasonable. However,
some gaps in the implementations are obvious, the existing clang
implementation disagrees with the Flang interpretation of it. SO, we're
keeping a task list here based on what gets discovered.

Changes to Clang:
- [x] Switch 'directive-kind' enum conversions to use tablegen See
ff1a7bd
- [x] Switch 'clause-kind' enum conversions to use tablegen See
ff1a7bd
- [x] Investigate 'parse' test differences to see if any new
disagreements arise.
- [x] Clang/Flang disagree as to whether 'collapse' can be multiple
times on a loop. Further research showed no prose to limit this, and the
comment on the clang implementation said "no good reason to allow", so
no standards justification.
- [x] Clang/Flang disagree whether 'num_gangs' can appear >1 on a
compute/combined construct. This ended up being an unjustified
restriction.
- [x] Clang/Flang disagree as to the list of required clauses on a 'set'
construct. My research shows that Clang mistakenly included 'if' in the
list, and that it should be just 'default_async', 'device_num', and
'device_type'.
- [x] Order of 'at least one of' diagnostic has changed. Tests were
updated.
- [x] Ensure we are properly 'de-aliasing' clause names in appertainment
checks?
- [x] What is 'shortloop'? 'shortloop' seems to be an old non-standard
extension that isn't supported by flang, but is parsed for backward
compat reasons. Clang won't parse, but we at least have a spot for it in
the clause list.
- [x] Implemented proposed change for 'routine' gang/worker/vector/seq.
(see issue 539)
- [x] Implement init/shutdown can only have 1 'if' (see issue 540)
- [x] Clang/Flang disagree as to whether 'tile' is permitted more than
once on a 'loop' or combined constructs (Flang prohibits >1). I see no
justification for this in the standard. EDIT: I found a comment in clang
that I did this to make SOMETHING around duplicate checks easier.
Discussion showed we should actually have a better behavior around
'device_type' and duplicates, so I've since implemented that.
- [x] Clang/Flang disagree whether 'gang', 'worker', or 'vector' may
appear on the same construct as a 'seq' on a 'loop' or 'combined'. There
is prose for this in 2022: (a gang, worker, or vector clause may not
appear if a 'seq' clause appears). EDIT: These don't actually disagree,
but aren't in the .td file, so I restored the existing code to do this.
- [x] Clang/Flang disagree on whether 'bind' can appear >1 on a
'routine'. I believe line 3096 (A bind clause may not bind to a routine
name that has a visible bind clause) makes this limitation (Flang
permits >1 bind). we discussed and decided this should have the same
rules as worker/vector/etc, except without the 'exactly 1 of' rule (so
no dupes in individual sections).
- [x] Clang/Flang disagree on whether 'init'/'shutdown' can have
multiple 'device_num' clauses. I believe there is no supporting prose
for this limitation., We decided that `device_num` should only happen
1x.
- [x] Clang/Flang disagree whether 'num_gangs' can appear >1 on a
'kernels' construct. Line 1173 (On a kernels construct, the num_gangs
clause must have a single argument) justifies limiting on a
per-arguement basis, but doesn't do so for multiple num_gangs clauses.
WE decided to do this with the '1-per-device-type' region for num_gangs,
num_workers, and vector_length, see openacc bug here:
https://github.com/OpenACC/openacc-spec/issues/541

Changes to Flang:
- [x] Clang/Flang disgree on whether 'atomic' can take an 'if' clause.
This was added in OpenACC3.3_Next See llvm#135451
- [x] Clang/Flang disagree on whether 'finalize' can be allowed >1 times
on a 'exit_data' construct. see llvm#135415.
- [x] Clang/Flang disagree whether 'if_present' should be allowed >1
times on a 'host_data'/'update' construct. see llvm#135422
- [x] Clang/Flang disagree on whether 'init'/'shutdown' can have
multiple 'device_type' clauses. I believe there is no supporting prose
for this limitation.
- [ ] SEE change for num_gangs/etc above.


Changes that need discussion/research:
IanWood1 pushed a commit to IanWood1/llvm-project that referenced this pull request May 6, 2025
llvm#135372)

…uses

The Flang implemenation of OpenACC uses a .td file in the llvm/Frontend
directory to determine appertainment in 4 categories:

-Required: If this list has items in it, the directive requires at least
1 of these be present.

-AllowedExclusive: Items on this list are all allowed, but only 1 from
the list may be here (That is, they are exclusive of eachother).

-AllowedOnce: Items on this list are all allowed, but may not be
duplicated.

Allowed: Items on this list are allowed. Note th at the actual list of
'allowed' is all 4 of these lists together.

This is a draft patch to swtich Clang over to use those tables. Surgery
to get this to happen in Clang Sema was somewhat reasonable. However,
some gaps in the implementations are obvious, the existing clang
implementation disagrees with the Flang interpretation of it. SO, we're
keeping a task list here based on what gets discovered.

Changes to Clang:
- [x] Switch 'directive-kind' enum conversions to use tablegen See
ff1a7bd
- [x] Switch 'clause-kind' enum conversions to use tablegen See
ff1a7bd
- [x] Investigate 'parse' test differences to see if any new
disagreements arise.
- [x] Clang/Flang disagree as to whether 'collapse' can be multiple
times on a loop. Further research showed no prose to limit this, and the
comment on the clang implementation said "no good reason to allow", so
no standards justification.
- [x] Clang/Flang disagree whether 'num_gangs' can appear >1 on a
compute/combined construct. This ended up being an unjustified
restriction.
- [x] Clang/Flang disagree as to the list of required clauses on a 'set'
construct. My research shows that Clang mistakenly included 'if' in the
list, and that it should be just 'default_async', 'device_num', and
'device_type'.
- [x] Order of 'at least one of' diagnostic has changed. Tests were
updated.
- [x] Ensure we are properly 'de-aliasing' clause names in appertainment
checks?
- [x] What is 'shortloop'? 'shortloop' seems to be an old non-standard
extension that isn't supported by flang, but is parsed for backward
compat reasons. Clang won't parse, but we at least have a spot for it in
the clause list.
- [x] Implemented proposed change for 'routine' gang/worker/vector/seq.
(see issue 539)
- [x] Implement init/shutdown can only have 1 'if' (see issue 540)
- [x] Clang/Flang disagree as to whether 'tile' is permitted more than
once on a 'loop' or combined constructs (Flang prohibits >1). I see no
justification for this in the standard. EDIT: I found a comment in clang
that I did this to make SOMETHING around duplicate checks easier.
Discussion showed we should actually have a better behavior around
'device_type' and duplicates, so I've since implemented that.
- [x] Clang/Flang disagree whether 'gang', 'worker', or 'vector' may
appear on the same construct as a 'seq' on a 'loop' or 'combined'. There
is prose for this in 2022: (a gang, worker, or vector clause may not
appear if a 'seq' clause appears). EDIT: These don't actually disagree,
but aren't in the .td file, so I restored the existing code to do this.
- [x] Clang/Flang disagree on whether 'bind' can appear >1 on a
'routine'. I believe line 3096 (A bind clause may not bind to a routine
name that has a visible bind clause) makes this limitation (Flang
permits >1 bind). we discussed and decided this should have the same
rules as worker/vector/etc, except without the 'exactly 1 of' rule (so
no dupes in individual sections).
- [x] Clang/Flang disagree on whether 'init'/'shutdown' can have
multiple 'device_num' clauses. I believe there is no supporting prose
for this limitation., We decided that `device_num` should only happen
1x.
- [x] Clang/Flang disagree whether 'num_gangs' can appear >1 on a
'kernels' construct. Line 1173 (On a kernels construct, the num_gangs
clause must have a single argument) justifies limiting on a
per-arguement basis, but doesn't do so for multiple num_gangs clauses.
WE decided to do this with the '1-per-device-type' region for num_gangs,
num_workers, and vector_length, see openacc bug here:
https://github.com/OpenACC/openacc-spec/issues/541

Changes to Flang:
- [x] Clang/Flang disgree on whether 'atomic' can take an 'if' clause.
This was added in OpenACC3.3_Next See llvm#135451
- [x] Clang/Flang disagree on whether 'finalize' can be allowed >1 times
on a 'exit_data' construct. see llvm#135415.
- [x] Clang/Flang disagree whether 'if_present' should be allowed >1
times on a 'host_data'/'update' construct. see llvm#135422
- [x] Clang/Flang disagree on whether 'init'/'shutdown' can have
multiple 'device_type' clauses. I believe there is no supporting prose
for this limitation.
- [ ] SEE change for num_gangs/etc above.


Changes that need discussion/research:
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flang:fir-hlfir flang:semantics flang Flang issues not falling into any other category openacc
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants